-
Notifications
You must be signed in to change notification settings - Fork 487
Feat: added context scope stream to enable streaming event from nested agent-as-tool invocations #697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.#contextScopeStreamOwner = true; | ||
| this.state._context._copyToContextScopeStream = true; | ||
| }, | ||
| cancel: () => {}, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Propagate cancellation for context scope stream
When streamAgentTools is enabled, the async iterator returned from StreamedRunResult is the context-scope stream created here, but its cancel handler is a no-op and never marks the result as cancelled. If a caller stops consuming the stream (e.g., breaks out of for await), the context stream is cancelled but #cancelled stays false, so _addItem continues to enqueue into a cancelled controller and the run loop keeps executing nested agents instead of short-circuiting. This regresses the previous cancellation semantics (line 264) and can throw once the controller is closed or waste work for abandoned streams. The cancel handler should set the same cancellation flag/cleanup as the primary stream.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Proposal
Introduce a new parameter
streamAgentToolsinStreamRunOptions. When set to true, theStreamRunResultobject returned byRunner.run()contains all stream events from current agent, and all agents invoked via agent-as-tools, recursively.For example, the following setup:
We can consume events by
for await (const event of result) {...}, and events originate from both agent A and agent B.Background
I was working on an app with a
mainagent using othertoolagents as tools, for better responsibility separation. To provide live update to end users, I process stream events from themainagent. In current implementation, the events frommainstream only consist oftool_called/tool_outputin run item events, or corresponding raw events.This behavior creates a challenge for live updates - end users can see that the
mainagent is invoking a tool, but it may look stuck iftoolagent takes a while to run. Although there are workarounds like writing updates in agent hooks, they all require code changes to handle this case. What I needed was a straightforward toggle to enable stream events from all agents recursively, working with agent-as-tools, handoffs, etc.I see similar PRs requesting for features in the Python library, so I feel that this is a pretty common use case.
Implementation details
I added a context-scope stream that all runners invoked with a single
RunContextcan use to send stream events. The first runner owns the stream, and other runner (initialized indirectly insideasTool()) copies events to the context-scope stream if necessary.Changes
streamAgentToolsinStreamRunOptionsStreamRunResultto handlestreamAgentTools; created context-scope stream; copied events to the context-scope stream; handled completion/abort/failure scenariosasToolexecute method to check run context; if enabled, passedStreamRunOptionsto runner and await for completionUnrelated fix
This PR also contains 2 commits to (1) remove redundant tool name replacement (non-alphanumeric -> underscore already handles space -> underscore), and (2) fix a typo (Schame -> Schema).